fix: harden server.json import review, URL elicitation scheme, and preview CSP injection by cliffhall · Pull Request #2876 · modelcontextprotocol/inspector

cliffhall · 2026-11-04T20:41:56Z

Fix for GHSA-rwv7-mh2c-ff2p, developed and reviewed in the advisory's private fork (inspector-GHSA-rwv7-mh2c-ff2p#1) and landed here as an ordinary PR. The commits are the fork branch's, unchanged.

Hardens three input-handling paths in the web client and the shared server.json import.

Fixes

server.json import shows the resolved launch before saving (core/mcp/import/serverJson.ts, useServerJsonImport.ts, ImportServerJsonPanel.tsx)
- The panel now shows the exact shell-quoted command line, or the remote URL, plus the env var names (never their values) for the selected option. This applies even when there is only one package and the selector is hidden. The text is built from the same buildServerConfigForSelection output that submit persists, so the preview can't drift from what gets saved.
- A package whose identifier or version starts with - or contains whitespace is skipped the same way an unsupported package is. A document with no other runnable option gets the existing "No runnable package or remote" error.
- runtimeArguments are deliberately left unfiltered. Registry runtime flags are legitimate (for example docker -v), so the fix makes them visible.
- New pure helper: clients/web/src/utils/commandLine.ts (quoteShellArg, formatCommandLine).
URL elicitation opens only http(s) (PendingClientRequestModal.tsx, ElicitationUrlPanel.tsx)
- Reuses the existing isHttpUrl from lib/downloadFile.ts.
- For a non-http(s) or unparseable URL, Open in Browser is disabled and a "URL refused" alert says why. The open handler re-checks before calling window.open. Copy URL stays enabled.
server.json env var names must be identifiers (core/mcp/import/serverJson.ts parseEnvVars)
- Each declared name must match ^[A-Za-z_][A-Za-z0-9_]*$. The registry schema types name as a bare string, so this is the POSIX shape. Without the check, a name like --privileged reaches docker as -e --privileged.
- An invalid name rejects the whole package, the same as a bad identifier or version, rather than dropping that one variable. Dropping it could silently remove a required var and launch a misconfigured server.
HTML preview CSP meta can no longer be spliced into a comment (contentViewerUtils.ts)
- wrapHtmlWithCsp no longer searches for <head> with a regex. It writes the meta as the document's first token, after a leading doctype if there is one, so the HTML parser always places it at the top of the head.
- Full documents keep their own head and <html> attributes. The input is not wrapped in a fixed <body>.

Tests

serverJson.test.ts:
- flag-shaped or whitespace-containing identifier/version are skipped across npm, pypi, oci and nuget
- a document whose only package is flag-shaped is rejected
- a legitimate docker -v runtime argument is preserved
- a docker package with --privileged as an env var name is skipped, falling back to the next option
- an npm package with an invalid env var name (1ABC, A B, A=B, -x) is rejected
- a valid name is still forwarded to docker as -e NAME
commandLine.test.ts: quoting rules.
useServerJsonImport.test.tsx:
- describeLaunch for stdio and remote configs
- the hook's launchPreview for a single package with a -c runtime argument
ImportServerJsonPanel.test.tsx: the command, URL and absent-preview cases.
ElicitationUrlPanel.test.tsx and PendingClientRequestModal.test.tsx: javascript:, data:, file: and unparseable URLs are refused, the button is disabled, the reason is shown, and window.open is never called.
contentViewerUtils.test.ts:
-  before the real head
- <head> inside a script or attribute
- doctype and BOM handling
- a comment before the doctype
- a bare fragment
ContentViewer.stories.tsx HtmlCommentedHead: a play function parses the wrapped  document in real Chromium. It asserts that the CSP meta is the head's first element and that the title, lang and body survive. happy-dom doesn't place a leading meta per the HTML spec, so this can't be asserted in the unit project.

npm run local:gate is green on this branch.

🤖 Generated with Claude Code

The registry server.json import panel labelled each option by its package identifier only, so the command and arguments that would actually be saved and spawned (runtime arguments included) were never shown before Add Server. - The panel now shows the exact shell-quoted command line (or remote URL) and the env var names of the selected option, including when there is only one package. It is built from the same buildServerConfigForSelection output that submit persists. - A package whose identifier or version starts with "-" or contains whitespace is skipped like an unsupported package, since either would reach the runtime as a flag rather than as the package reference. Runtime arguments themselves are unchanged: registry runtimeArguments are legitimate (e.g. docker -v), so they are surfaced rather than filtered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

The URL elicitation modal passed the server-supplied URL to window.open without checking its scheme. It now uses the shared isHttpUrl allowlist: a non-http(s) URL disables Open in Browser and the panel says why, and the open handler re-checks before calling window.open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

wrapHtmlWithCsp located the first <head> with a regex, which can match a <head> inside a comment, script or attribute and splice the policy where the parser never applies it. The meta is now written as the first token (after a leading doctype, if any), so the parser always places it at the top of the head, ahead of any of the document's own markup. Full documents keep their own head and <html> attributes; the input is not wrapped in a fixed body. A Storybook play function asserts the parsed placement in a real browser, since happy-dom does not place a leading meta the way the HTML spec does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

A declared environment variable name is passed to docker run as `-e NAME` for OCI packages, so a name such as `--privileged` would be read as a docker flag. parseEnvVars now requires each name to match ^[A-Za-z_][A-Za-z0-9_]*$ (the registry schema types it as a bare string, so this is the POSIX shape). Behavior chosen: an invalid name rejects the whole package, the same way a flag-shaped identifier or version does, rather than dropping that one variable. Dropping it could silently remove a required variable and launch a misconfigured server; rejecting it leaves the document with the existing "No runnable package or remote" error when nothing else is runnable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

Copilot

🟡 Changes recommended

Critical issues allow external requests despite the preview CSP and let immediate submissions save runtime arguments that were never shown in the preview.

3 open issues

What changed in this PR

This PR addresses #2873 by hardening server.json imports, URL elicitation, and HTML previews across the shared core and web client.

Changes:

Displays resolved launches before import and rejects unsafe package tokens and invalid environment-variable names.
Restricts elicitation browser actions to HTTP(S), with visible refusal messages.
Moves preview CSP insertion ahead of document markup and adds unit, integration, and browser regression coverage.

File	Description
`core/mcp/import/serverJson.ts`	Validates package tokens and environment-variable names.
`clients/web/src/utils/commandLine.ts`	Adds shell-quoted command formatting.
`clients/web/src/utils/commandLine.test.ts`	Tests argument quoting and command formatting.
`clients/web/src/test/integration/mcp/import/serverJson.test.ts`	Covers invalid imports and preserved runtime flags.
`clients/web/src/hooks/useServerJsonImport.ts`	Builds resolved launch previews.
`clients/web/src/hooks/useServerJsonImport.test.tsx`	Tests command and remote previews.
`clients/web/src/components/groups/ServerImportJsonModal/ServerImportJsonModal.tsx`	Passes the preview into the import panel.
`clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.tsx`	Guards browser opening with HTTP(S) validation.
`clients/web/src/components/groups/PendingClientRequestModal/PendingClientRequestModal.test.tsx`	Tests refusal of unsupported and malformed URLs.
`clients/web/src/components/groups/ImportServerJsonPanel/ImportServerJsonPanel.tsx`	Displays commands, remote URLs, and environment names.
`clients/web/src/components/groups/ImportServerJsonPanel/ImportServerJsonPanel.test.tsx`	Tests launch-preview rendering.
`clients/web/src/components/groups/ImportServerJsonPanel/ImportServerJsonPanel.stories.tsx`	Adds preview data to import stories.
`clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.tsx`	Displays refusal reasons and disables browser opening.
`clients/web/src/components/groups/ElicitationUrlPanel/ElicitationUrlPanel.test.tsx`	Tests refusal messaging and copy availability.
`clients/web/src/components/elements/ContentViewer/contentViewerUtils.ts`	Replaces head matching with leading CSP insertion.
`clients/web/src/components/elements/ContentViewer/contentViewerUtils.test.ts`	Covers markup, doctype, and fragment insertion cases.
`clients/web/src/components/elements/ContentViewer/ContentViewer.stories.tsx`	Adds a browser-parser regression for commented heads.

_{Review effort: Balanced}

Any doctype other than a plain <!DOCTYPE html> preceded by HTML whitespace now gets the meta written at the very start of the document. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

Add is disabled, and submit refuses, while the live text differs from the debounced text the launch preview was built from. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

cliffhall · 2026-11-04T21:10:26Z

Copilot round 1 (review 5775536740): three findings, fixed in 87ab65c, ddc3bed and 521dd31 and answered in each thread.

Preview CSP and the doctype (87ab65c). A doctype stays ahead of the CSP meta only when it is a plain <!DOCTYPE html>: HTML whitespace only, plus an optional BOM. Any other prefix, including PUBLIC/SYSTEM identifiers, gets the meta at the very start of the document. New Chromium Storybook play-function stories: HtmlDoctypeQuotedSystem, HtmlDoctypeQuotedPublic and HtmlDoctypeAfterNbsp.
- The quoted-> case did not reproduce in Chromium, which ends the doctype at that > as the spec says.
- The old regex did have a real gap: a U+00A0 before the doctype left the meta in the body, where it is not applied. HtmlDoctypeAfterNbsp fails on the old code and passes now.
Add during the debounce (ddc3bed). Add is disabled, and submit() refuses, until the live text and the debounced text match. A new hook test submits a valid-to-valid edit inside the debounce window.
Empty env-var name (521dd31). An entry is skipped only when name is missing or not a string. An explicit "" now rejects the package. Both cases are in the integration test.

local:gate passed (exit 0).

Copilot

🟢 Approval recommended

There are no active review issues requiring resolution for the input-handling hardening changes.

No open issues 🎉

3 resolved since last review

_{Review effort: Balanced}

cliffhall · 2026-11-04T21:14:30Z

Copilot review loop closed. Round 2 (review 5775895557) was clean: no inline comments, no open issues, and the three round-1 findings are marked resolved.

cliffhall and others added 4 commits October 29, 2026 00:10

cliffhall added the v2 Issues and PRs for v2 label Nov 4, 2026

cliffhall linked an issue Nov 4, 2026 that may be closed by this pull request

[GHSA-rwv7-mh2c-ff2p] - Unvalidated server.json import args, elicitation URL scheme, preview CSP injection #2873

Closed

cliffhall requested a balanced review from Copilot November 4, 2026 20:42

Copilot started reviewing on behalf of cliffhall November 4, 2026 20:43

View session

cliffhall mentioned this pull request Nov 4, 2026

security-advisory skill: land GHSA fixes through ordinary public PRs, not private-fork merges #2880

Closed

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread clients/web/src/components/elements/ContentViewer/contentViewerUtils.ts Resolved

Comment thread clients/web/src/hooks/useServerJsonImport.ts Resolved

Comment thread core/mcp/import/serverJson.ts Resolved

cliffhall and others added 3 commits November 4, 2026 15:56

fix(import): reject a server.json env var with an empty name

521dd31

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>

cliffhall requested a balanced review from Copilot November 4, 2026 21:10

Copilot started reviewing on behalf of cliffhall November 4, 2026 21:10

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

cliffhall mentioned this pull request Nov 4, 2026

[GHSA-rwv7-mh2c-ff2p] - Unvalidated server.json import args, elicitation URL scheme, preview CSP injection #2873

Closed

cliffhall merged commit e01fef3 into v2/main Nov 4, 2026
11 checks passed

cliffhall deleted the v2/fix/2873-server-json-import-hardening branch November 4, 2026 21:38

This was referenced Nov 4, 2026

Release 2.14.0: merge v2/main into main again for the security fixes #2881

Closed

chore(release): merge v2/main into main for the v2.14.0 security fixes #2882

Merged

Navigation Menu

Repository navigation

fix: harden server.json import review, URL elicitation scheme, and preview CSP injection - #2876

fix: harden server.json import review, URL elicitation scheme, and preview CSP injection#2876
cliffhall merged 7 commits into
v2/mainfrom
v2/fix/2873-server-json-import-hardening

cliffhall commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cliffhall commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

cliffhall commented Nov 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cliffhall commented Nov 4, 2026

Fixes

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cliffhall commented Nov 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

cliffhall commented Nov 4, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants